Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FEAT/#10] 번호 인증 검사 기능 구현 #15

Merged
merged 32 commits into from
Dec 5, 2024
Merged

[FEAT/#10] 번호 인증 검사 기능 구현 #15

merged 32 commits into from
Dec 5, 2024

Conversation

yummygyudon
Copy link
Member

Related Issue 🚀


Work Description ✏️

  • 번호 인증 검사 기능을 구현했습니다.

(Jpa 테스트의 경우, 리뷰를 받는 과정에서 추가 진행할 예정입니다.)


PR Point 📸

※ Verify(검증) 메서드 역할 분리

  • 현재 구조
    • verify() 메서드 구현체 내에서 데이터 삭제까지 진행
    • 요청 데이터 - 조회 데이터가 동일할 경우, 조회 데이터에 대해 삭제하는 로직이 한 메서드에 포함되어 있습니다.
  • 고민 구조
    1. Usecase 내 delete() 메서드 추가 구현하여 함수 분리
    2. (현재 구조 유지)

@sung-silver @hyunw9
위 사항에 대한 여러분들의 의견이 궁금합니다!! 가감없이 의견 공유 주세요!!


※ Usercase 메서드 반환 타입

본 PR에서는 구현한 verify() 메서드의 반환 타입으로 AuthResponse.VerifyResult DTO 객체를 반환하여
해당 반환 객체를 Response Body Data로 주입하도록 구현했습니다.

본래 계층 간 협력 관계에서 협력 메시지 객체를 사용하도록 하여 도메인 객체를 외부로부터 보존하기 위해 위 방법을 채택했습니다!

이 때, 아래와 같은 상황이 우려되었습니다.
" 한 개의 API 메서드 내부에서 복수의 Usecase 메서드를 호출하는 경우 "


Usecase의 메서드 반환 타입이 모두 Response Body DTO 객체라면
복수의 Usecase 메서드를 호출했을 때, " 복수의 Response Body DTO를 재조합하여 새로운 Response Body DTO 객체를 생성 "해야하는 상황이 발생할 것으로 예상되었습니다.

@sung-silver @hyunw9
이러한 상황은 바람직한 구조는 아니라고 판단되어 여러분들과 논의를 해보면 좋을 것 같아 가져오게 되었습니다!!
편하게 의견 공유 주세요!!

@yummygyudon
[FEAT] 번호 인증 검사 API Response Body 데이터 정의
d8b0159
@yummygyudon
[FEAT] 번호 인증 Usecase 및 Command 정의
a4c06ce 
Return 타입으로 Response DTO를 선언했습니다.
@yummygyudon
[FEAT] 번호 인증 데이터 Out Port 정의 및 JPA 메서드 선언
270684e
@yummygyudon
[FEAT] 번호 인증 검사 In Port Usecase 구현
e57a769 
요청 데이터 기반 도메인 객체와 조회 데이터 기반 도메인 객체의 동일여부에 따라 검증 여부를 판별할 수 있도록 합니다.
- PhoneVerification 객체에 대해 `@EqualsAndHashCode` 어노테이션을 추가하여 필드값을 통한 비교가 가능하도록 했습니다.

* AuthRequest 객체에서 Command로 변환하는 메서드도 정의했습니다.
* 누락된 Repository 내 인증 이력 조회가 없을 경우에 대한 실패 코드 정의 작업 내용을 추가했습니다.
@yummygyudon
[FEAT] 번호 인증 요청에 대한 API 메서드 구현
72cf821
@yummygyudon
[TEST] PhoneVerification 객체 간 동일여부 판단 메서드 테스트
0ab5f54
@yummygyudon
[FIX] PhoneVerification Inner Record @EqualsAndHashCode 추가
9d0c2f2 
- 내부 `PhoneVerification.VerificationCode` Record 또한 객체로서 생성되어 HashCode 값이 상이하기 때문에 발생한 테스트 불통과 문제를 해결했습니다.
@yummygyudon yummygyudon added the 🎁 feature 새로운 기능을 개발하거나 추가, 변경할 경우(spring boot 내의 기능 코드) label Nov 18, 2024
@yummygyudon yummygyudon self-assigned this Nov 18, 2024
@yummygyudon yummygyudon linked an issue Nov 18, 2024 that may be closed by this pull request
[FEAT] 전화번호 인증 - 검사 기능 구현 #10
Closed
1 task
@height Height
Copy link

height bot commented Nov 18, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@yummygyudon
[REFACTOR] Transaction Helper 객체 구현
557048a 
기존 interface의 default 접근제어자 메서드들에 접근 가능한 환경을 개선했습니다.
- 구현 클래스가 많아지기 때문에 내부적으로 도메인별 패키징을 추가했습니다.
@yummygyudon
[REFACTOR] PhoneVerification Out Port 메서드명 개선
87b7deb 
Transaction의 성격이 강한 메서드 명보다 직관적인 메서드 명으로 변경했습니다.
@yummygyudon
[CHORE] 소스코드 변경 사항 및 패키징 추가 반영
b134dda
@yummygyudon
[CHORE] 와일드 카드 제거 및 Usecase 반환 데이터 변경 반영
83c80bf
@hyunw9
Copy link
Contributor

hyunw9 commented Nov 19, 2024

코드 리뷰는 오프라인으로 진행했기 때문에 생략하겠슴다~~~

1.

제 생각에는요 useCase 자체가 행위 기반으로 이루어진 집합이기 때문에, delete와 같은 추가적인 메서드를 사용하는 것 보다
현재의 방식이 더 적합하다고 생각합니다 !

다만 verify 메서드에서 두 가지의 역할을 하고 있음이 분명하기 때문에, SRP를 유지가 목적이라면 Event 발행 구조를 적용해도 될 것 같지만
이벤트 발행을 해도, 어차피 한 트랜잭션에 묶여서 처리되기 때문에 가독성 향상의 이점 빼고는 크게 얻어가는 부분이 없다고 생각합니다.

따라서 제 의견은 현행 유지. 입니다.

2.

만약 한 API 엔드포인트에서 여러 유스케이스를 호출하는 구조라면, 유즈케이스 설계가 잘못된 경우라고 생각합니다!
저는 VO 객체로 반환하는 편이 더 낫다고 생각합니다.

hyunw9
hyunw9 approved these changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@hyunw9 hyunw9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생했슴둥~~

sung-silver
sung-silver requested changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@sung-silver sung-silver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

궁금한 점 남겨보았습니다!!

  1. verify 메서드 역할 분리
  • 가독성을 생각하면 delete()로 분리해도 좋을 것 같습니다!
  • delete로 분리하더라도 verify내에서 호출되면 두 가지의 일을 하는건 동일하다는 느낌? ㅎ
  1. 한 개의 API 메서드 내부에서 복수의 Usecase 메서드를 호출하는 경우
  • Usecase -> 서비스의 인터페이스인데 두 개의 dto를 조합하여 데이터를 만들어내는 경우는 없지 않을지..! 조심스레 걱정 안해도 될 것 같다고 생각합니다!

src/main/java/sopt/makers/authentication/application/auth/dto/response/AuthResponse.java Outdated Show resolved Hide resolved
src/main/java/sopt/makers/authentication/database/PhoneVerificationRepositoryImpl.java Show resolved Hide resolved
src/main/java/sopt/makers/authentication/database/PhoneVerificationRepositoryImpl.java Show resolved Hide resolved
.../sopt/makers/authentication/database/rdb/repository/auth/PhoneVerificationJpaRepository.java Outdated Show resolved Hide resolved
src/main/java/sopt/makers/authentication/usecase/auth/service/VerifyVerificationService.java Show resolved Hide resolved
@yummygyudon
[REFACTOR] 트랜젝션 처리/예외 처리 계층 변경
a23cbf4 
helper 레이어를 추가함으로서 기존 Repository 인터페이스 내에서 default 메서드 정의로 예외 처리하던 구조를 helper 객체로 이관했습니다.
- Repository 객체는 실제 쿼리 실행 역할만 수행할 수 있도록 개선했습니다.
- [리뷰](#15 (comment))
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Dec 4, 2024
@yummygyudon
Merge branch 'dev' into feat/#10
af48afe
@yummygyudon
[FEAT] User 도메인 Out Port 추가 - UserRegisterInfo Repository 및 UserRegis…
064e5a9 
…terInfo 도메인 정의

- 전화번호 기반 조회 메서드를 선언합니다.
@yummygyudon
[FEAT] UserRegisterInfo 데이터 계층(external) 객체 구현
74c56cd 
- UserRegisterInfoEntity의 경우, UserRegisterInfo 도메인 객체로 변환하는 메서드 정의까지 했습니다.
@yummygyudon
[FEAT] User 데이터 계층(external) 전화번호 기반 조회 기능 구현
3414ee5 
- 데이터가 존재하지 않는 경우에 대한 UserFailure 까지 구현했습니다.
@yummygyudon
[MODIFY] 사용자 정보가 존재할 경우에만 인증 이력이 생성되도록 기능 수정
d6e3e29 
- `REGISTER` 타입일 경우, "동아리 가입자 정보" 데이터 베이스를 조회하도록 합니다.
- `CHANGE`/`SEARCH` 타입의 경우, "기존 가입자 정보" 데이터 베이스를 조회하도록 합니다.
@yummygyudon
[MODIFY] 인증 완료 시, 대상자 이름 및 번호 데이터를 함께 반환할 수 있도록 DTO 수정
c642ca2 
- 이에 따른 AuthApiControllerTest 코드까지 수정 반영했습니다.
@yummygyudon
[MODIFY] 인증 처리 Usecase Return 방식 VO 반환 방식으로의 변경에 따른 반영
6a5a651 
- 구현체 VerifyVerificationServiceTest 내 코드 반영까지 완료했습니다.
@yummygyudon
[TEST] 인증 생성 UseCase 구현체 Test
83488cf 
Verification Type에 따라 올바른 대상에 대한 조회 기반으로 처리되는지 확인합니다.
sung-silver
sung-silver approved these changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@sung-silver sung-silver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다~

@yummygyudon yummygyudon merged commit fb26534 into dev Dec 5, 2024
@sung-silver sung-silver changed the title [FEAT] 번호 인증 검사 기능 구현 [FEAT/#10] 번호 인증 검사 기능 구현 Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sung-silver sung-silver sung-silver approved these changes

@hyunw9 hyunw9 hyunw9 approved these changes

Assignees

@yummygyudon yummygyudon

Labels
🎁 feature 새로운 기능을 개발하거나 추가, 변경할 경우(spring boot 내의 기능 코드) size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEAT] 전화번호 인증 - 검사 기능 구현 [FEAT] 전화번호 인증 기능 구현
3 participants
@yummygyudon @hyunw9 @sung-silver